SFR-2239: Remove error codes from error page #536
Conversation
jackiequach
requested review from
samanthaandrews,
kristojorg,
Toxiapo and
oliviawongnyc
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/components/Feedback/Feedback.tsx
Outdated
| @@ -38,7 +44,7 @@ const Feedback: React.FC<any> = ({ location }) => { | |||
| values: React.ComponentProps<typeof FeedbackBox>["onSubmit"] | |||
| ) => { | |||
| submitFeedback({ | |||
| feedback: values.comment, | |||
| feedback: isError ? `${statusCode} - ${values.comment}` : values.comment, | |||
There was a problem hiding this comment.
Looks like statusCode could be null though I hope that's never the case so we may just want to have a default. What are your thoughts?
{statusCode ?? 'Unknown'} - ${values.comment}
There was a problem hiding this comment.
We may also want to prefix the status code with something like Error Code: or HTTP Status Code: just so folks understand what the status code means!
There was a problem hiding this comment.
Yes, good idea!
There was a problem hiding this comment.
Can you add some before/after screenshots when you get the chance? Super helpful for the reviewer to quickly spot check the design changes. Thanks!
The vercel link https://sfr-bookfinder-front-end-git-sfr-2239-remove-error-codes-nypl.vercel.app/ can't find the home page but the error page looks good.
| feedback: isError | ||
| ? `Error Code: ${statusCode ?? "Unknown"} - ${values.comment}` | ||
| : values.comment, |
| heading: "Something went wrong on our end.", | ||
| subText: "We encountered an error while trying to load the page. ", | ||
| tryText: "Try refreshing the page or ", | ||
| }, | ||
| 404: { | ||
| overline: "error 404", | ||
| heading: "We couldn't find that page.", | ||
| subText: | ||
| "The page you were looking for doesn't exist or may have moved elsewhere. ", | ||
| tryText: "Try a different URL or ", | ||
| }, | ||
| 400: { |
There was a problem hiding this comment.
May want to consider having a default error map.
src/pages/_error.tsx
Outdated
| width={100} | ||
| /> | ||
| <Text size="overline1">{errorMap[statusCode].overline}</Text> | ||
| <Heading marginTop="s" marginBottom="s"> | ||
| {errorMap[statusCode].heading} |
There was a problem hiding this comment.
Here you may just want guard against unexpected error codes like 502, 503, 403 and go to the default error mapping.
There was a problem hiding this comment.
Spoke with Apoorva and she recommended that we use the 400 error as default.
Jira Ticket
Figma Designs
This PR does the following:
Testing requirements & instructions:
Before (Desktop):
After (Desktop):
Before (Mobile):
After (Mobile):
